Two queries for keyword script field #59527
Conversation
This adds the `exits` and `terms` query for the `keyword` style of scripted field.
nik9000
added
the
:Search/Search
|
Pinging @elastic/es-search (:Search/Search) |
|
I picked these two queries to do next because they are fairly simple and prove that we can add more queries. I haven't tried to share any code between the queries yet because I figure it'll be more obvious what we can and should share once we have a few more queries. |
.../src/main/java/org/elasticsearch/xpack/runtimefields/query/StringScriptFieldExistsQuery.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public String bareToString() { | ||
| return "*"; |
There was a problem hiding this comment.
I get it, but I find this cryptic. For instance looking at what DocValuesFieldExistsQuery does , that is clearer to me.
There was a problem hiding this comment.
I can keep the original toString impls.
| } | ||
| StringScriptFieldTermsQuery other = (StringScriptFieldTermsQuery) obj; | ||
| return other.terms.equals(other.terms); | ||
| } |
There was a problem hiding this comment.
should we have unit tests for equals/hashcode , toString and visit for all the queries that we introduce?
There was a problem hiding this comment.
possibly also matches but that is already tested
There was a problem hiding this comment.
Yeah. equals and hashcode are cache keys. toString probably isn't all that important but we may as well do it while we're there. Visit is important for highlighting, or, at least, it will be. So we should do it too.
|
|
||
| @Override | ||
| public int hashCode() { | ||
| // TODO should leafFactory be here? Something about the script probably should be! |
There was a problem hiding this comment.
good point. what do we need to address this TODO? Would we need to pass in the Script itself?
| if (fieldName().contentEquals(field)) { | ||
| return "*"; | ||
| } | ||
| return fieldName() + ":*"; |
There was a problem hiding this comment.
I thought we said we would adapt toString for the exists query to be something more aligned with DocValuesFieldExistsQuery in lucene
| } | ||
| }); | ||
| assertThat(allTerms, equalTo(query.terms().stream().map(t -> new Term(query.fieldName(), t)).collect(toCollection(TreeSet::new)))); | ||
| } |
There was a problem hiding this comment.
thanks for adding all these tests!!!
This adds the
existsandtermsquery for thekeywordstyle ofscripted field.
relates to #59332